Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Overview subsection of the explanation section #915

Merged
merged 27 commits into from
Sep 21, 2021
Merged

Overview subsection of the explanation section #915

merged 27 commits into from
Sep 21, 2021

Conversation

nibble0101
Copy link
Contributor

Improve the Introduction and Explanation sections project

Section Sub-section Technical Writer
Explanations Overview of the explanation section Chris Estepa

This is Overview sub-section of the Explanations section

image
image
image

@nibble0101 nibble0101 requested a review from a team as a code owner June 1, 2021 08:44
@nibble0101 nibble0101 added explanation Pull requests that update explanation section of the documentation GSoD 2021 labels Jun 1, 2021
@nibble0101 nibble0101 mentioned this pull request Jun 1, 2021
Open
Rohitesh-Kumar-Jain
Rohitesh-Kumar-Jain approved these changes Jun 2, 2021
View reviewed changes
Copy link
Contributor

@Rohitesh-Kumar-Jain Rohitesh-Kumar-Jain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @nibble0101 and @chris-4444 : )

proudofsimin
proudofsimin approved these changes Jun 3, 2021
View reviewed changes
Copy link
Contributor

@proudofsimin proudofsimin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@lijiarui
Copy link
Member

hi @Rohitesh-Kumar-Jain As GSoD Project PR Review Rules in https://github.com/wechaty/wechaty.js.org/issues/1045

This PR should follow our rules and get enough approval, then I will review this PR. So I remove the ready to merge tag and feel free to add this tag when it gets enough approval.

@nibble0101 nibble0101 requested a review from chris-4444 July 12, 2021 10:45
@Rohitesh-Kumar-Jain
Copy link
Contributor

@chris-4444 I would like you to once go through the PR, and approve when you think it's ready to get merged.

chris-4444
chris-4444 approved these changes Jul 13, 2021
View reviewed changes
Copy link
Contributor

@chris-4444 chris-4444 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@nibble0101 nibble0101 mentioned this pull request Jul 15, 2021
Open
@Rohitesh-Kumar-Jain Rohitesh-Kumar-Jain mentioned this pull request Jul 18, 2021
Closed
lijiarui
lijiarui requested changes Jul 18, 2021
View reviewed changes
Copy link
Member

@lijiarui lijiarui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems explanation overview is talking about chatbot not Wechaty.

I think we should add more Wechaty part instead of chatbot part.

docusaurus/docs/explainations/overview.mdx Outdated Show resolved Hide resolved
docusaurus/docs/explainations/overview.mdx Outdated Show resolved Hide resolved
@nibble0101 nibble0101 requested a review from lijiarui July 21, 2021 13:48
@chris-4444
Copy link
Contributor

@lijiarui May I know what do you mean by adding more about the Wechaty part? Is it pertaining to Wechaty as an organization itself? If yes, it's already covered in What is Wechaty and What can you do with Wechaty. In any case that the information we've provided is lacking, would it be okay if you explain it to us in more detail? Kindly advise so that we can revise. Thanks a lot! 😉

@lijiarui
Copy link
Member

@chris-4444

May I know what do you mean by adding more about the Wechaty part

May I know where is this comment related to this PR?

huan
huan approved these changes Sep 7, 2021
View reviewed changes
Copy link
Member

@huan huan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Please read and follow #1257, and feel free to help yourself to merge it by following the merge workflow.

Thank you very much!

@chris-4444
Copy link
Contributor

chris-4444 commented Sep 9, 2021 •
edited
Loading

@lijiarui @huan @Rohitesh-Kumar-Jain kindly merge because merging is disabled for me and @nibble0101. thanks.

@huan
Copy link
Member

huan commented Sep 9, 2021 •
edited
Loading

@chris-4444 The reason that you can not merge this PR is that @lijiarui has a "request changes" on it.

Please contact @lijiarui and get the approval from her, then this PR will be good to go.

@chris-4444
Copy link
Contributor

@lijiarui i have updated the post accordingly. kindly approve. thanks.

@huan
Copy link
Member

huan commented Sep 10, 2021

ping @lijiarui

lijiarui
lijiarui requested changes Sep 14, 2021
View reviewed changes
Copy link
Member

@lijiarui lijiarui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part looks a little bit strange, some of the explanation parts is shown while the other part isn't shown. Could you tell us the reason why you design like this?

docusaurus/docs/explanations/overview.mdx Outdated
@@ -1,7 +1,27 @@
---
slug: /explanations/
title: Wechaty explanations overview
sidebar_label: 'explanations: Overview'
title: Overview of Explanation section
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

It seems the title is too long here, I suggest changing to Overview as the other part wrote.

Also, I think we should change explanation to Explanation as the other part wrote, using the uppercase letter.

chris-4444
chris-4444 approved these changes Sep 14, 2021
View reviewed changes
Copy link
Contributor

@chris-4444 chris-4444 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shortened the SDK title as requested.

@nibble0101 nibble0101 requested a review from a team as a code owner September 14, 2021 14:41
lijiarui
lijiarui requested changes Sep 15, 2021
View reviewed changes
docusaurus/docs/explanations/overview.mdx Outdated Show resolved Hide resolved
docusaurus/docs/explanations/overview.mdx Outdated Show resolved Hide resolved
Rohitesh-Kumar-Jain
Rohitesh-Kumar-Jain approved these changes Sep 16, 2021
View reviewed changes
Copy link
Contributor

@Rohitesh-Kumar-Jain Rohitesh-Kumar-Jain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested by Rui have been implemented 👍🏼

lijiarui
lijiarui approved these changes Sep 18, 2021
View reviewed changes
Copy link
Member

@lijiarui lijiarui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nibble0101 nibble0101 merged commit 5e3271d into wechaty:master Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chris-4444 chris-4444 chris-4444 approved these changes

@Rohitesh-Kumar-Jain Rohitesh-Kumar-Jain Rohitesh-Kumar-Jain approved these changes

@proudofsimin proudofsimin proudofsimin approved these changes

@lijiarui lijiarui lijiarui approved these changes

@huan huan huan approved these changes

Assignees

@chris-4444 chris-4444

Labels
explanation Pull requests that update explanation section of the documentation GSoD 2021 ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nibble0101 @lijiarui @Rohitesh-Kumar-Jain @chris-4444 @huan @proudofsimin